[Accordion] New Accordion components and hooks #577

mj12albert · 2024-09-03T06:43:15Z

Closes #25
Builds on top of:

Preview

👉 https://deploy-preview-577--base-ui.netlify.app/components/react-accordion

Summary

orientation: 'vertical' | 'horizontal': sets a data attr and navigates focus between triggers with ArrowRight/Left instead of ArrowDown/Up
Collapsible.Content now exposes the --collapsible-content-width var to support horizontal orientation
direction: 'ltr' | 'rtl': sets the dir attr like other components and reverses ArrowRight & ArrowLeft when orientation === 'horizontal'
openMultiple: boolean: default true, controls whether multiple sections can be open at the same time, not expected to change during the lifetime of the component
disabled can be set on the Root, Section, or Trigger
hidden="until-found" can be set on the Root or a Panel
useCollapsibleTrigger (and Collapsible/AccordionTrigger) now uses useButton

Extra demos

Animations + hidden="until-found" https://deploy-preview-577--base-ui.netlify.app/experiments/accordion-animations
Horizontal LTR + RTL https://deploy-preview-577--base-ui.netlify.app/experiments/accordion-horizontal

mui-bot · 2024-09-03T06:46:05Z

Netlify deploy preview

https://deploy-preview-577--base-ui.netlify.app/

Generated by 🚫 dangerJS against 80b04e0

docs/data/components/accordion/accordion.mdx

docs/data/components/accordion/UnstyledAccordionIntroduction.tsx

packages/mui-base/src/Accordion/Heading/AccordionHeading.tsx

michaldudak · 2024-09-06T07:51:51Z

packages/mui-base/src/Accordion/Heading/AccordionHeading.test.tsx

+  describeConformance(<Accordion.Heading />, () => ({
+    inheritComponent: 'h3',
+    render: (node) => {
+      const { container, ...other } = render(


I saw this pattern in a couple of your components. Why not just return render(...)?

It was just copy-pasta 🙈

packages/mui-base/src/Accordion/Heading/AccordionHeading.test.tsx

packages/mui-base/src/Accordion/Heading/AccordionHeading.tsx

packages/mui-base/src/Accordion/Root/useAccordionRoot.ts

packages/mui-base/src/index.ts

Fix uncontrolled mode Fix broken links

michaldudak · 2024-09-20T07:52:11Z

packages/mui-base/src/Accordion/Header/AccordionHeader.tsx

+  const { ownerState } = useAccordionItemContext();
+
+  const { renderElement } = useComponentRenderer({
+    render: render ?? 'h3',


h3 seems arbitrary here. I wonder if a div won't be a safer option.

michaldudak · 2024-09-20T07:52:32Z

packages/mui-base/src/Accordion/Header/AccordionHeader.tsx

+  props: AccordionHeader.Props,
+  forwardedRef: React.ForwardedRef<HTMLHeadingElement>,
+) {
+  const { render, className, ...otherProps } = props;


For consistency

Suggested change

const { render, className, ...otherProps } = props;

const { render, className, ...other } = props;

michaldudak · 2024-09-20T07:53:04Z

packages/mui-base/src/Accordion/Header/AccordionHeader.tsx

+export { AccordionHeader };
+
+export namespace AccordionHeader {
+  export interface Props extends BaseUIComponentProps<'h3', AccordionItem.OwnerState> {}
+}


Suggested change

export { AccordionHeader };

export namespace AccordionHeader {

export interface Props extends BaseUIComponentProps<'h3', AccordionItem.OwnerState> {}

}

namespace AccordionHeader {

export interface Props extends BaseUIComponentProps<'h3', AccordionItem.OwnerState> {}

}

export { AccordionHeader };

other files

michaldudak · 2024-09-20T07:53:41Z

packages/mui-base/src/Accordion/Header/AccordionHeader.test.tsx

+// import { expect } from 'chai';
+// import { spy } from 'sinon';


Suggested change

// import { expect } from 'chai';

// import { spy } from 'sinon';

michaldudak · 2024-09-20T07:54:21Z

packages/mui-base/src/Accordion/Header/AccordionHeader.test.tsx

+  const { render } = createRenderer();
+
+  describeConformance(<Accordion.Header />, () => ({
+    inheritComponent: 'h3',


inheritComponent is not used anymore. You can remove it from all the tests.

michaldudak · 2024-09-20T07:55:16Z

packages/mui-base/src/Accordion/Item/AccordionItem.tsx

+    onOpenChange: onOpenChangeProp,
+    render,
+    value: valueProp,
+    ...otherProps


Suggested change

...otherProps

...other

michaldudak · 2024-09-20T07:56:04Z

packages/mui-base/src/Accordion/Item/AccordionItem.tsx

+    ...otherProps
+  } = props;
+
+  const sectionRef = React.useRef<HTMLElement>(null);


Seems like this ref is unused

michaldudak · 2024-09-20T07:56:57Z

packages/mui-base/src/Accordion/Item/AccordionItem.tsx

+    if (onOpenChangeProp) {
+      onOpenChangeProp(nextOpen);
+    }


Suggested change

if (onOpenChangeProp) {

onOpenChangeProp(nextOpen);

}

onOpenChangeProp?.(nextOpen);

michaldudak · 2024-09-20T07:58:22Z

packages/mui-base/src/Accordion/Panel/AccordionPanel.test.tsx

+// import { expect } from 'chai';
+// import { spy } from 'sinon';


Suggested change

// import { expect } from 'chai';

// import { spy } from 'sinon';

michaldudak · 2024-09-20T08:33:03Z

packages/mui-base/src/Accordion/index.ts

@@ -0,0 +1,10 @@
+export { AccordionRoot as Root } from './Root/AccordionRoot';


Do not export hooks and contexts. We may revisit them in the future, but for now we decided to export just the components

mj12albert added new feature New feature or request component: accordion This is the name of the generic UI component, not the React module! labels Sep 3, 2024

mj12albert mentioned this pull request Sep 3, 2024

[Accordion] New Accordion components and hooks #551

Closed

1 task

mj12albert force-pushed the feat/accordion branch 4 times, most recently from 36e29e3 to 881476c Compare September 3, 2024 15:02

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 4, 2024

mj12albert force-pushed the feat/accordion branch 2 times, most recently from 84b0482 to e16cad0 Compare September 4, 2024 05:33

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 4, 2024

mj12albert force-pushed the feat/accordion branch 7 times, most recently from bb181bf to c735efb Compare September 4, 2024 07:49

mj12albert marked this pull request as ready for review September 4, 2024 07:50

mj12albert requested review from atomiks, colmtuite and michaldudak as code owners September 4, 2024 07:50

mj12albert force-pushed the feat/accordion branch 3 times, most recently from b5f818e to 6969cd2 Compare September 5, 2024 05:11

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 5, 2024

mj12albert force-pushed the feat/accordion branch from 6969cd2 to 91333ff Compare September 5, 2024 09:25

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 5, 2024

mj12albert force-pushed the feat/accordion branch from 91333ff to 85062eb Compare September 5, 2024 09:33

mj12albert force-pushed the feat/accordion branch 4 times, most recently from f9e05b1 to fc907ba Compare September 6, 2024 05:27

michaldudak reviewed Sep 6, 2024

View reviewed changes

colmtuite requested a review from vladmoroz September 9, 2024 08:56

mj12albert force-pushed the feat/accordion branch 4 times, most recently from 93364ca to 9f531a6 Compare September 16, 2024 10:58

mj12albert added 10 commits September 17, 2024 14:43

Provide Collapsible content width css var

a64950b

New Accordion hooks and components

ab49d59

Add docs and demos

12353ff

Misc fixes

adb4ce2

Fix uncontrolled mode Fix broken links

Refactor useCollapsibleTrigger with useButton

357b1f5

Fix review comments

a211d7b

Rename subcomponents

895872c

Support focus loop

9605d9f

Add tests

bfc09a1

Rename data-state for Accordion and Collapsible

3c687fa

mj12albert force-pushed the feat/accordion branch from 9f531a6 to 3c687fa Compare September 17, 2024 06:56

Use a boolean prop to apply hidden-until-found

28978bb

mj12albert force-pushed the feat/accordion branch from a5475e4 to 28978bb Compare September 17, 2024 07:14

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 18, 2024

colmtuite approved these changes Sep 18, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/master' into feat/accordion

c16a64a

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 19, 2024

mj12albert requested a review from michaldudak September 20, 2024 05:18

Merge remote-tracking branch 'upstream/master' into feat/accordion

80b04e0

michaldudak reviewed Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Accordion] New Accordion components and hooks #577

[Accordion] New Accordion components and hooks #577

mj12albert commented Sep 3, 2024 •

edited

Loading

mui-bot commented Sep 3, 2024 •

edited

Loading

michaldudak Sep 6, 2024

mj12albert Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

michaldudak Sep 20, 2024

	const { render, className, ...otherProps } = props;
	const { render, className, ...other } = props;

		// import { expect } from 'chai';
		// import { spy } from 'sinon';

		@@ -0,0 +1,10 @@
		export { AccordionRoot as Root } from './Root/AccordionRoot';

[Accordion] New Accordion components and hooks #577

Are you sure you want to change the base?

[Accordion] New Accordion components and hooks #577

Conversation

mj12albert commented Sep 3, 2024 • edited Loading

Preview

Summary

Extra demos

mui-bot commented Sep 3, 2024 • edited Loading

Netlify deploy preview

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mj12albert commented Sep 3, 2024 •

edited

Loading

mui-bot commented Sep 3, 2024 •

edited

Loading